Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for JWT authentication DS-959 #206

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

BryanttV
Copy link
Contributor

Description

This PR allows you to make a request with a JWT token.

Testing instructions

Using Tutor:

  1. Install the plugin with these changes. If you are installing it for the first time you should run the migrations.

  2. Create a new application from http://local.edly.io:8000/admin/oauth2_provider/application/. Associate a user with sufficient permissions and add to the Redirect uris field: http://local.edly.io:8000/

  3. Generate a JWT Token for the user using the next endpoint. POST http://local.edly.io:8000/oauth2/access_token/. The content type of the request must be application/x-www-form-urlencoded

    Body parameters

    • client_id: Client ID of the application created above. e.g. CAfZv...IHD0J
    • client_secret: Client Secret of the application created above. e.g. Qu2pnJJ9b1C6DA...u7Ej8x31RFiaKg9
    • grant_type: client_credentials
    • token_type: JWT

    Response

      {
          "access_token": "eyJhbGciOiJIUz...SsDAs3KE_gEc",
          "expires_in": 3600,
          "token_type": "JWT",
          "scope": "read write email profile"
      }
    • access_token: The user's access token. You must use this token in the Authorization header of the requests to the API.
  4. Test all eox-tenant endpoints using the JWT token, all should work correctly. You can use this eox-tenant postman collection.

Checklist for Merge

  • Tested in a remote environment
  • Updated documentation
  • Rebased master/main
  • Squashed commits

@Alec4r Alec4r requested review from Alec4r and Asespinel May 30, 2024 21:09
@mariajgrimaldi mariajgrimaldi requested a review from a team May 31, 2024 00:35
bra-i-am
bra-i-am previously approved these changes May 31, 2024
Copy link
Contributor

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@MaferMazu
Copy link
Contributor

I will test this soon, but in the meantime, can you add the Postman collection to the repository? Create a docs directory, something like https://github.com/eduNEXT/eox-core/tree/master/docs/resources, to add the collection you already created. It doesn't enter explicitly in the scope but will be helpful for future testing, and you already have the collection. Is it possible?

@BryanttV
Copy link
Contributor Author

BryanttV commented May 31, 2024

Hi @MaferMazu. Yes, I can do something similar to how it is in eox-core.

MaferMazu
MaferMazu previously approved these changes May 31, 2024
Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as expected! Thanks @BryanttV.

If you could add the collection, that would be great.

By the way, we usually use the squash and merge button to merge PRs.

image

When you squash and merge, the bump version workflow will run automatically (it will create a new tag with the latest version of eox-tenant depending on the commit message you use; with the feat, you will have a minor version). You only need to squash and merge and then go to https://github.com/eduNEXT/eox-tenant/releases and publish the latest release if we need it (if it is a change that does not require a release (is a change that does not modify the functionality or is not required for fixing a bug), you don't need to publish it). This will change slightly in the following sprints, but it works as I said for now. If you have questions, please don't hesitate to ask.

On the other hand, before merging, you will need to sign your commits. Ref: https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

@BryanttV BryanttV dismissed stale reviews from MaferMazu and bra-i-am via 25c56aa May 31, 2024 14:42
@github-actions github-actions bot added size/l and removed size/m labels May 31, 2024
@mariajgrimaldi
Copy link
Collaborator

@MaferMazu: do we deploy the latest release somewhere for remote testing?

@BryanttV BryanttV force-pushed the bav/add-jwt-support branch from 25c56aa to 3684e57 Compare May 31, 2024 16:25
@BryanttV
Copy link
Contributor Author

@MaferMazu, I already added the postman collection in docs/resources, and I also left a single signed commit.

@MaferMazu
Copy link
Contributor

@mariajgrimaldi No, we only test this in our environments for now. Then, we want to implement something to make testing easy in remote environments.

Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks @BryanttV 🙌

@BryanttV BryanttV merged commit 40a72eb into master Jun 4, 2024
4 checks passed
@mariajgrimaldi mariajgrimaldi linked an issue Aug 27, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

["FEAT"] Add JWT Authentication support
6 participants